Skip to content

feature-skip-preprocess-on-restart - #1761

Merged
sbryngelson merged 7 commits into
MFlowCode:masterfrom
danieljvickers:feature-skip-preprocess-on-restart
Aug 30, 2026
Merged

feature-skip-preprocess-on-restart#1761
sbryngelson merged 7 commits into
MFlowCode:masterfrom
danieljvickers:feature-skip-preprocess-on-restart

Conversation

@danieljvickers

Copy link
Copy Markdown
Member

I have deleted valuable restart data one-to-many times because I forget to call -t simulation on restart. This PR makes the adjustment that pre_process must be explicitly called on restart. This still allows one to restart cases with preprocess if they desire to modify patches, but prevents the deletion of all data from the directory because one forgets to set a target.


Acknowledgement

  • I confirm this PR meets the above expectations and reflects my own understanding and real-world context.

PR template credit: junegunn

…d on restart. This is a preventative measure against deleting all of your restart data.
@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

Claude Code Review

Head SHA: 1c52a66

Files changed:

  • 4
  • docs/documentation/case.md
  • toolchain/mfc/args.py
  • toolchain/mfc/lint_docs.py
  • toolchain/mfc/run/run.py

Findings:

  • toolchain/mfc/args.py:92 detects an explicit -t/--targets only via exact token equality (tok in ("-t", "--targets")). This misses the very common --targets=pre_process form and short concatenated forms like -tpre_process, both valid argparse syntaxes. When a user restarts a case (t_step_start/n_start > 0) and explicitly requests --targets=pre_process, targets_explicit in toolchain/mfc/run/run.py:160 comes back False, so the new auto-skip logic at toolchain/mfc/run/run.py:164-166 silently strips pre_process from targets anyway — directly contradicting the explicit request the feature is supposed to honor (and the doc text added in docs/documentation/case.md promising -t pre_process "forces it"). This is a silent behavior change (pre_process quietly not run) rather than a crash, which is the class of bug this codebase's tests are least likely to catch.

@sbryngelson

Copy link
Copy Markdown
Member

lol. check ai comments when you have a chance

sbryngelson
sbryngelson previously approved these changes Aug 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents default preprocessing from overwriting restart data.

Changes:

  • Detects explicitly requested run targets.
  • Skips preprocessing for restart runs by default.
  • Documents the behavior and updates documentation linting.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
toolchain/mfc/run/run.py Adds restart-aware preprocessing exclusion.
toolchain/mfc/args.py Records explicit target selection.
toolchain/mfc/lint_docs.py Allows the documented target name.
docs/documentation/case.md Documents restart behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread toolchain/mfc/args.py
Comment thread toolchain/mfc/run/run.py Outdated
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.67%. Comparing base (fbb3de2) to head (1c52a66).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1761   +/-   ##
=======================================
  Coverage   61.67%   61.67%           
=======================================
  Files          84       84           
  Lines       21619    21619           
  Branches     3196     3196           
=======================================
  Hits        13334    13334           
  Misses       6093     6093           
  Partials     2192     2192           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@sbryngelson
sbryngelson marked this pull request as draft August 27, 2026 14:19
@sbryngelson
sbryngelson marked this pull request as ready for review August 28, 2026 23:40
@sbryngelson
sbryngelson merged commit 2ba5c23 into MFlowCode:master Aug 30, 2026
146 of 157 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants